Skip to content

Provide APD access over thin client#2737

Closed
GabrieleManduchi wants to merge 1 commit intoalphafrom
gm-fix-apd-python
Closed

Provide APD access over thin client#2737
GabrieleManduchi wants to merge 1 commit intoalphafrom
gm-fix-apd-python

Conversation

@GabrieleManduchi
Copy link
Contributor

Rebased on Alpha, introduces the management of APD access over thin client for python. Checked also against older mdsip servers.

@zack-vii
Copy link
Contributor

Is this not already handled by getObject

def getObject(self, exp, *args, **kwargs):

@GabrieleManduchi
Copy link
Contributor Author

getObject() forces serialization/deserialization. get() and put() are more general and apply serialization only when required, and transparently to the user, keeping at the same time compatibility with the old mdsip servers and presenting the same interface and semantic of C++ and Java (they do not have getObject()).

@zack-vii
Copy link
Contributor

zack-vii commented Apr 10, 2024

why not adding a putObject() which would do serialization under the hood instead of tempering with get and put
something like:
self.get("TreePut($, `serializein($))", node, data.serialize()) i dont see how this is less transparent.
My point is changeing get and put to do serialization by default is a big change and not transparent at all, where as explicit method getObject() and putObject() that do the same as get() and put() would get you the same functionality without changing the behavior of existing methods.

@GabrieleManduchi
Copy link
Contributor Author

There is indeed not a real need in the python interface to change the behavior of get() and put() , being getObject() also available (unlike C++ and Java). The real need was in Java because of the use of MATLAB structured data.
For me we can also keep things as they are, but we lose uniformity across different languages.

@zack-vii
Copy link
Contributor

so why not adding getObject to the other interfaces instead.

@GabrieleManduchi
Copy link
Contributor Author

Not feasible without a big effort. The reason is that MATLAB access at TCV goes through thin client via get() and they need full support for MATLAB (arrays of) structures. Defining getObject() in Java and going through it would require deeply changing all the MATLAB and Java support. It took a lot of time have it working and sincerely I prefer rather leaving things as they are now.

@GabrieleManduchi
Copy link
Contributor Author

Forgot to say: and in any case the equivalent of putObject is missing in the alpha version. In this branch, put() sends serialized data only if APD are present.

@GabrieleManduchi
Copy link
Contributor Author

Sorry, things come in my mind once a time....the reason why get() performs serialization is that we may not know in advance whether the result of the expression is an APD or not. So it is not possible to know in advance whether using getObject() or get() when retrieving data in thin client

@zack-vii
Copy link
Contributor

I get that .. hence you would use getObject() and putObject() for you new code but allow older software to use get() and put() as usual. In particular you should not make APD into a special case as there is a whole class of R descriptors that require serialization.. even legacy float types would require serialization if you wish to preserve the original dtype. it is only due to backward compatibility why we have this workaround. ideally we would add a getObject() putObject() to mdsipshr that would always use serialization for passed args and returned value. this way we dont need to re-implement it in all languages individually. I developed a very flexible implementation in java mdsplus-api.. the beauty. it is as well only client site. it defines helper methods that can easily wrap arbitrary user expressions with arguments into a fully serialized form. A variation of it would do good in the core c-library. it should have a signature derived from TdiExecute() however with additional context info (holding connection info and tree info).

@GabrieleManduchi
Copy link
Contributor Author

It is a bit more complicated.....there are use cases in which the type of the data being read is not known in advance, but discovered after having been read it (e.g. when introspecting some data). This means that we don't know whether using get() or getObject() in advance. This is the reason why get() has been made in this branch general enough to be able to read any data, including APD.

@WhoBrokeTheBuild
Copy link
Member

After working with the internals of the python API and thin client a lot over the past few weeks, I second Timo's comments. I think we should provide new methods for accomplishing these tasks, but I don't like the idea of serializing all .get()'s and some .put()'s by default.

@WhoBrokeTheBuild
Copy link
Member

Closing due to inactivity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants